Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: bump CAPI to v1.6.x #1115

Merged
merged 5 commits into from
Feb 2, 2024
Merged

Conversation

damdo
Copy link
Member

@damdo damdo commented Jan 22, 2024

What type of PR is this?

What this PR does / why we need it:

This bumps main to use v1.6.1 of CAPI. It also bumps to use Go v1.21 and controller-runtime v0.16.3.

NOTES:
Note for the reviewers: the apidiff is failing because I removed (the already deprecated) v1alpha3 as that has been deprecated by the core cluster-api project in 1.6.x and we depended on that in CAPG /api/v1alpha3.

TODOs:

  • squashed commits
  • includes documentation
  • adds unit tests

Release note:

Bump CAPI to v1.6.x (and associated packages)

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jan 22, 2024
@k8s-ci-robot k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jan 22, 2024
Copy link
Member

@cpanato cpanato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/ok-to-test
/assign

@k8s-ci-robot k8s-ci-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Jan 22, 2024
@damdo
Copy link
Member Author

damdo commented Jan 22, 2024

@cpanato like so? kubernetes/test-infra#31690

@damdo
Copy link
Member Author

damdo commented Jan 22, 2024

/retest

@damdo
Copy link
Member Author

damdo commented Jan 22, 2024

@cpanato looks like that didn't help much

@cpanato
Copy link
Member

cpanato commented Jan 22, 2024

i will take a look this week

@damdo
Copy link
Member Author

damdo commented Jan 22, 2024

It looks like that's because even the 1.28 image uses go 1.20.x

$ docker run --rm -it --entrypoint=/bin/bash gcr.io/k8s-staging-test-infra/kubekins-e2e:v20240111-cf1d81388e-1.28
root@c04a6e08bc89:/workspace# go version
go version go1.20.13 linux/amd64

https://github.com/kubernetes/test-infra/blob/7bbc8c4e93f30230ca0200ede47e6e88033d9385/images/kubekins-e2e/variants.yaml#L42

@cpanato Maybe we should try using the 1.29 one, which uses GO_VERSION: 1.21.6?

@damdo
Copy link
Member Author

damdo commented Jan 22, 2024

It looks like using 1.29 images is what CAPA is also doing for CAPI 1.6.x.
@cpanato I opened kubernetes/test-infra#31691 to use those for CAPG as well, PTAL.

@damdo
Copy link
Member Author

damdo commented Jan 22, 2024

/retest

@damdo damdo force-pushed the bump-capi-1.6.x branch 6 times, most recently from 2c95dd9 to ee3aba2 Compare January 23, 2024 13:36
@damdo
Copy link
Member Author

damdo commented Jan 23, 2024

Hit rate limit
/test pull-cluster-api-provider-gcp-e2e-test

@damdo
Copy link
Member Author

damdo commented Jan 23, 2024

Hmm deployment "capg-controller-manager" is not ready after 5m0s it looks like the CAPG manager deployment is not coming up correctly.
It'd be nice to be able to check the pod logs/events.
Is there a way to do so @cpanato ? Thanks!

@damdo
Copy link
Member Author

damdo commented Jan 24, 2024

/test ?

@k8s-ci-robot
Copy link
Contributor

@damdo: The following commands are available to trigger required jobs:

  • /test pull-cluster-api-provider-gcp-build
  • /test pull-cluster-api-provider-gcp-e2e-test
  • /test pull-cluster-api-provider-gcp-make
  • /test pull-cluster-api-provider-gcp-test
  • /test pull-cluster-api-provider-gcp-verify

The following commands are available to trigger optional jobs:

  • /test pull-cluster-api-provider-gcp-apidiff
  • /test pull-cluster-api-provider-gcp-capi-e2e
  • /test pull-cluster-api-provider-gcp-conformance
  • /test pull-cluster-api-provider-gcp-conformance-ci-artifacts
  • /test pull-cluster-api-provider-gcp-coverage
  • /test pull-cluster-api-provider-gcp-coverage-release-1-4
  • /test pull-cluster-api-provider-gcp-coverage-release-1-5
  • /test pull-cluster-api-provider-gcp-e2e-workload-upgrade

Use /test all to run the following jobs that were automatically triggered:

  • pull-cluster-api-provider-gcp-apidiff
  • pull-cluster-api-provider-gcp-build
  • pull-cluster-api-provider-gcp-e2e-test
  • pull-cluster-api-provider-gcp-make
  • pull-cluster-api-provider-gcp-test
  • pull-cluster-api-provider-gcp-verify

In response to this:

/test ?

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

if err != nil {
return nil
}

return pointer.String(parsed.ID()) //nolint: staticcheck
return ptr.To[string](parsed.ID()) //nolint: staticcheck
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return ptr.To[string](parsed.ID()) //nolint: staticcheck
return ptr.To(parsed.ID()) //nolint: staticcheck

All of these aren't needed, they're implicit

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah you are right, although it's nice to have typing assertion there. I think it might be worth keeping them

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 30, 2024
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 30, 2024
@damdo
Copy link
Member Author

damdo commented Jan 31, 2024

@cpanato @vincepri @richardcase I ran E2Es locally, found the issue and fixed them 🎉
This is ready for labelling now. Thanks!

@damdo
Copy link
Member Author

damdo commented Jan 31, 2024

Note for the reviewers: the apidiff is failing because I removed (the already deprecated) v1alpha3 as that has been deprecated by the core cluster-api project in 1.6.x and we depended on that in CAPG /api/v1alpha3.

@cpanato
Copy link
Member

cpanato commented Jan 31, 2024

/test ls

@k8s-ci-robot
Copy link
Contributor

@cpanato: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

  • /test pull-cluster-api-provider-gcp-build
  • /test pull-cluster-api-provider-gcp-e2e-test
  • /test pull-cluster-api-provider-gcp-make
  • /test pull-cluster-api-provider-gcp-test
  • /test pull-cluster-api-provider-gcp-verify

The following commands are available to trigger optional jobs:

  • /test pull-cluster-api-provider-gcp-apidiff
  • /test pull-cluster-api-provider-gcp-capi-e2e
  • /test pull-cluster-api-provider-gcp-conformance
  • /test pull-cluster-api-provider-gcp-conformance-ci-artifacts
  • /test pull-cluster-api-provider-gcp-coverage
  • /test pull-cluster-api-provider-gcp-coverage-release-1-4
  • /test pull-cluster-api-provider-gcp-coverage-release-1-5
  • /test pull-cluster-api-provider-gcp-e2e-workload-upgrade

Use /test all to run the following jobs that were automatically triggered:

  • pull-cluster-api-provider-gcp-apidiff
  • pull-cluster-api-provider-gcp-build
  • pull-cluster-api-provider-gcp-e2e-test
  • pull-cluster-api-provider-gcp-make
  • pull-cluster-api-provider-gcp-test
  • pull-cluster-api-provider-gcp-verify

In response to this:

/test ls

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@cpanato
Copy link
Member

cpanato commented Jan 31, 2024

/test pull-cluster-api-provider-gcp-conformance

@cpanato
Copy link
Member

cpanato commented Jan 31, 2024

@cpanato @vincepri @richardcase I ran E2Es locally, found the issue and fixed them 🎉 This is ready for labelling now. Thanks!

cool great job!!!

@cpanato
Copy link
Member

cpanato commented Jan 31, 2024

/test pull-cluster-api-provider-gcp-capi-e2e

1 similar comment
@damdo
Copy link
Member Author

damdo commented Jan 31, 2024

/test pull-cluster-api-provider-gcp-capi-e2e

@k8s-ci-robot
Copy link
Contributor

@damdo: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-cluster-api-provider-gcp-apidiff d067593 link false /test pull-cluster-api-provider-gcp-apidiff
pull-cluster-api-provider-gcp-capi-e2e d067593 link false /test pull-cluster-api-provider-gcp-capi-e2e

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@damdo
Copy link
Member Author

damdo commented Feb 1, 2024

@cpanato to check whether pull-cluster-api-provider-gcp-capi-e2e has been broken by this PR, or already was before, I opened a No-op PR to check it here

EDIT: it looks like it fails on "main" with the same issue, hence unrelated to this PR.

@damdo
Copy link
Member Author

damdo commented Feb 1, 2024

@cpanato updated the comment above with the results of my test

@cpanato
Copy link
Member

cpanato commented Feb 2, 2024

I see, thanks for the investigation, lets move forward with this one and try to fix that test in a follow up, and then after that we can cut a release

thanks so much for working on this

/approve
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 2, 2024
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cpanato, damdo

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 2, 2024
@k8s-ci-robot k8s-ci-robot merged commit 0d60ffb into kubernetes-sigs:main Feb 2, 2024
15 of 17 checks passed
@k8s-ci-robot k8s-ci-robot added this to the v1.1.0 milestone Feb 2, 2024
@damdo damdo mentioned this pull request Feb 2, 2024
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants